Add Ruby cucumber-query implementation by mattwynne · Pull Request #167 · cucumber/query

mattwynne · 2026-05-14T22:46:37Z

🤔 What's changed?

🤖 Matt's robot here — this PR adds a Ruby implementation of cucumber-query under ruby/:

a publishable cucumber-query gem scaffold with README, gemspec, Rake, RSpec and RuboCop wiring
a Cucumber::Query::Query API that consumes Cucumber Messages envelopes and indexes the message stream
the query methods currently needed by the Ruby pretty-formatter work, aligned with the existing JavaScript query API where practical
shared cucumber-query testdata acceptance coverage for the Ruby implementation, plus a few focused unit specs for Ruby edge cases

⚡️ What's your motivation?

The Ruby pretty-formatter implementation needs a Ruby cucumber-query package instead of building a private query layer inside the formatter. This PR provides that dependency so Ruby formatters can consume the same message stream model as the other Cucumber implementations.

The implementation is deliberately scoped to the methods exercised by the shared testdata and the pretty-formatter branch. It should be extended from shared fixtures as new formatter/query needs appear.

🏷️ What kind of change is this?

⚡ New feature (non-breaking change which adds new behaviour)
🏦 Refactoring/debt/DX (adds Ruby package/test tooling to this polyglot repo)

♻️ Anything particular you want feedback on?

Whether the Ruby API surface and packaging shape fit the polyglot cucumber-query repository.
Whether any methods should be renamed or held back before this becomes a published Ruby gem.

Provenance note: this is not Luke's old skeleton branch. It was implemented for the Ruby pretty-formatter branch against the current main branch, using the JavaScript implementation as the behavioural reference and the shared cucumber-query testdata as the executable contract.

📋 Checklist:

I agree to respect and uphold the Cucumber Community Code of Conduct
I've changed the behaviour of the code
- I have added/updated tests to cover my changes.
My change requires a change to the documentation.
- I have updated the documentation accordingly.
Users should know about my change
- I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

mpkorstanje · 2026-05-15T01:52:57Z

@mattwynne would you mind following the pull request template you made? It is kinda useful.

Also keep the audience in mind. The "Test evidence" section isn't really informative, I can see that in CI output myself. Likewise the "Summary" just tells me which files you changed. I can see those too. What I am missing however is the "What's your motivation?". That I have to infer from context clues.

mpkorstanje

Cheers!

The shape looks alright. But I suspect you've used an LLM to some degree so that doesn't mean anything. 😉

For example I see that you've implemented/copied update_gherkin_document from the JavaScript implementation, but find_location_of(pickle) isn't tested. And you wouldn't have to as pickles have a location since cucumber/messages#308.

So now I'm a bit puzzled. I thought Luke had a test driven skeleton version of Query. And I would have expected to see that in this PR. But right now I'm looking at a lot of material with a unknown provenance. I'd rather not spend too much time reviewing statistically plausible code.

mpkorstanje · 2026-05-15T01:59:24Z

+      end
+
+      def timestamp_ms(timestamp)
+        (timestamp.seconds * 1_000) + (timestamp.nanos / 1_000_000.0)


Is this integer or float division?

<Matt's robot>
Updated in f01d586: this now uses timestamp.nanos.fdiv(1_000_000) so the nanoseconds-to-milliseconds conversion is explicitly floating point.
</Matt's robot>

mpkorstanje · 2026-05-15T02:01:37Z

The recommendation for YAML files is to use the .yaml extension.

Please also copy over the the release-rubygem.yaml from any other project.

<Matt's robot>
Updated in f01d586: renamed the Ruby test workflow to .github/workflows/test-ruby.yaml. The RubyGems release workflow is already present as .github/workflows/release-rubygem.yaml, copied in this PR.
</Matt's robot>

mpkorstanje · 2026-05-15T02:02:20Z

+      end
+
+      def duration_from_ms(milliseconds)
+        seconds = (milliseconds / 1_000).floor


Is this integer or float division?

<Matt's robot>
Updated in f01d586: this path now uses milliseconds.div(1_000) to make the whole-seconds part explicitly integer division. The fractional remainder is then converted back to nanos.
</Matt's robot>

mpkorstanje · 2026-05-15T02:05:10Z

+      def find_all_test_case_started
+        @test_case_started_by_id.values
+                                .reject { |started| @test_case_finished_by_test_case_started_id[started.id]&.will_be_retried }
+                                .sort_by { |started| [timestamp_ms(started.timestamp), started.id] }


This doesn't look incredibly efficient. Wouldn't a comparator make more sense?

<Matt's robot>
This code has moved into ResultOrder. I kept the decorate/sort/undecorate shape intentionally: it computes each order key exactly once, whereas a comparator would call the find_order_by query repeatedly during sorting. Happy to revisit if you prefer the comparator shape for consistency with another implementation.
</Matt's robot>

mpkorstanje · 2026-05-15T02:05:52Z

+        @test_case_finished_by_test_case_started_id.values
+                                                   .reject(&:will_be_retried)
+                                                   .sort_by do |finished|
+          [timestamp_ms(finished.timestamp),


This doesn't look incredibly efficient. Wouldn't a comparator make more sense?

<Matt's robot>
Same as the previous ordering thread: this now lives in ResultOrder, and the current approach avoids recomputing the sort key inside a comparator.
</Matt's robot>

mpkorstanje · 2026-05-15T02:14:42Z

+      end
+
+      def update(envelope)
+        envelopes << envelope


Is this necessary?

<Matt's robot>
Updated in f01d586: removed this guard. The indexer now assumes callers pass Cucumber::Messages::Envelope objects.
</Matt's robot>

mpkorstanje · 2026-05-15T02:16:20Z

+
+      def update(envelope)
+        envelopes << envelope
+        return nil unless envelope.respond_to?(:meta)


Is this a type check of sorts? It should be true for any envelope, but at the same time doesn't tell you if an object is an envelope.

<Matt's robot>
Updated in f01d586: removed the respond_to?(:meta) envelope-ish type check and changed the unit spec to pass a real Cucumber::Messages::Envelope.
</Matt's robot>

mpkorstanje · 2026-05-15T02:20:44Z

+  s.authors     = ['Cucumber Limited']
+  s.description = 'Given one Cucumber Message, find another'
+  s.summary     = "cucumber-query-#{s.version}"
+  s.email       = 'cukes@googlegroups.com'


Some fields look a bit bit out of date. 😉

<Matt's robot>
Updated in f01d586: refreshed the gemspec author name, description, and metadata, including adding changelog_uri.
</Matt's robot>

real Matt here: what are we supposed to use for those contact details?

I updated it to use my name and hello@cucumber.io

mattwynne · 2026-05-15T04:10:50Z

🤖 Matt's robot here — thanks for the review. I've updated the PR description to follow the template and make the motivation/provenance clearer. I also pushed 40021d5 to simplify find_location_of to use the pickle's own location from cucumber-messages, so it no longer depends on the Gherkin document lineage for that case. Shared Ruby query acceptance coverage still passes locally with bundle exec rake (270 examples, 0 failures; RuboCop 15 files, no offenses).

mattwynne · 2026-05-15T04:37:14Z

Actual Matt here! — totally LLM driven @mpkorstanje! Thanks for taking the time to read through it and review. I'll work through all your comments.

davidjgoss · 2026-05-15T06:47:40Z

+
+Because Cucumber Messages refer to related data by id, feed the complete stream
+to the query when possible. If a formatter asks for a pickle before the
+corresponding `pickle` message has been seen, the lookup will return `nil`.


On this, it would be good to update the table in https://github.com/cucumber/query/blob/main/CONTRIBUTING.md#types, this gives good hints for when somebody wants to add a new method across languages.

So, like, an additional column for Ruby with T | nil, Array and Hash?

mpkorstanje · 2026-05-15T09:35:36Z

Slop.

mattwynne mentioned this pull request May 14, 2026

Add Ruby cucumber-pretty-formatter implementation cucumber/pretty-formatter#105

Closed

7 tasks

mattwynne force-pushed the ruby-implementation branch 3 times, most recently from a10f47d to 7757538 Compare May 14, 2026 23:00

mpkorstanje requested changes May 15, 2026

View reviewed changes

mattwynne requested a review from mpkorstanje May 15, 2026 04:12

mattwynne added 5 commits May 14, 2026 23:22

Add Ruby cucumber-query gem scaffold

2fee02e

Implement Ruby query API

12ae4b0

Cover Ruby query with shared acceptance data

643a0d2

Document the Ruby query package

7c4e54d

Add Ruby package CI and release workflows

610c8cb

mattwynne force-pushed the ruby-implementation branch from 5188d5f to 610c8cb Compare May 15, 2026 06:23

davidjgoss reviewed May 15, 2026

View reviewed changes

mpkorstanje marked this pull request as draft May 15, 2026 08:59

mpkorstanje closed this May 15, 2026

luke-hill deleted the ruby-implementation branch June 30, 2026 16:11

Uh oh!

Uh oh!

Conversation

mattwynne commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤔 What's changed?

⚡️ What's your motivation?

🏷️ What kind of change is this?

♻️ Anything particular you want feedback on?

📋 Checklist:

Uh oh!

mpkorstanje commented May 15, 2026

Uh oh!

mpkorstanje left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattwynne commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattwynne commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpkorstanje commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mattwynne commented May 14, 2026 •

edited

Loading

mpkorstanje left a comment •

edited

Loading

mattwynne commented May 15, 2026 •

edited

Loading

mattwynne commented May 15, 2026 •

edited

Loading